Skip to content

[WinSDK] Modularize <guiddef.h> #83914

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[WinSDK] Modularize <guiddef.h> #83914

wants to merge 1 commit into from
+60 −10

Conversation

egorzhdan
Copy link
Contributor

This fixes modularization errors that arise when importing a C++ header that contains #include <guiddef.h>, which might hijack this header from the WinSDK module where it belongs.

@egorzhdan egorzhdan requested a review from compnerd August 26, 2025 11:04
@egorzhdan egorzhdan requested a review from a team as a code owner August 26, 2025 11:04
@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test

@egorzhdan
Copy link
Contributor Author

Ugh, looks like this header is under /shared and not /um.

@compnerd
Copy link
Member

Yeah, the GUID definition is shared between um and km. We currently get it through transitive includes.

@egorzhdan egorzhdan force-pushed the egorzhdan/guiddef branch 2 times, most recently from 6388790 to 862b799 Compare August 28, 2025 11:19
egorzhdan
egorzhdan commented Aug 28, 2025
View reviewed changes
lib/ClangImporter/ClangIncludePaths.cpp Outdated
@@ -493,6 +493,17 @@ void GetWindowsFileMappings(
if (!AuxiliaryFile.empty())
fileMapping.redirectedFiles.emplace_back(std::string(WinSDKInjection),
AuxiliaryFile);

if (WindowsSDK.MajorVersion > 8) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still support older SDKs? This might be a bit simpler if we don't.

Copy link
Member

@compnerd compnerd Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we do support older SDKs, which is why this gets tricky, but only from 10.0.17763 (Windows 10 RS5).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so we should be able to remove this check then.

compnerd
compnerd reviewed Aug 28, 2025
View reviewed changes
Runtimes/Overlay/Windows/clang/CMakeLists.txt Outdated
@@ -48,4 +54,5 @@ install(FILES
vcruntime.apinotes
vcruntime.modulemap
winsdk.modulemap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should rename this to winsdk_um.modulemap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable! Renamed it.

@egorzhdan
[WinSDK] Modularize <guiddef.h>
28f9db7 
This fixes modularization errors that arise when importing a C++ header that contains `#include <guiddef.h>`, which might hijack this header from the WinSDK module where it belongs.
egorzhdan added a commit to swiftlang/swift-installer-scripts that referenced this pull request Aug 29, 2025
@egorzhdan
[WinSDK] Adjust the installer for platform modulemaps
04c60fc 
This is a companion to swiftlang/swift#83914.
@egorzhdan egorzhdan mentioned this pull request Aug 29, 2025
Open
@egorzhdan
Copy link
Contributor Author

swiftlang/swift-installer-scripts#458

@swift-ci please test Windows

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test macOS

@egorzhdan
Copy link
Contributor Author

@swift-ci please smoke test Linux

@egorzhdan
Copy link
Contributor Author

swiftlang/swift-installer-scripts#458

@swift-ci please build toolchain Windows ARM64

@grynspan grynspan mentioned this pull request Sep 2, 2025
Open
@grynspan grynspan added the Windows Platform: Windows label Sep 2, 2025
@egorzhdan
Copy link
Contributor Author

swiftlang/swift-installer-scripts#458

@swift-ci please build toolchain Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compnerd compnerd compnerd left review comments

@zoecarver zoecarver Awaiting requested review from zoecarver zoecarver is a code owner

@beccadax beccadax Awaiting requested review from beccadax beccadax is a code owner

@ian-twilightcoder ian-twilightcoder Awaiting requested review from ian-twilightcoder ian-twilightcoder is a code owner

@Xazax-hun Xazax-hun Awaiting requested review from Xazax-hun Xazax-hun is a code owner

@j-hui j-hui Awaiting requested review from j-hui j-hui is a code owner

@fahadnayyar fahadnayyar Awaiting requested review from fahadnayyar fahadnayyar is a code owner

@susmonteiro susmonteiro Awaiting requested review from susmonteiro susmonteiro is a code owner

@etcwilde etcwilde Awaiting requested review from etcwilde etcwilde is a code owner

@edymtt edymtt Awaiting requested review from edymtt edymtt is a code owner

@justice-adams-apple justice-adams-apple Awaiting requested review from justice-adams-apple justice-adams-apple is a code owner

Assignees
No one assigned
Labels
Windows Platform: Windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@egorzhdan @compnerd @grynspan